Skip to content

Conversation

@pawosm-arm
Copy link
Contributor

When linking with other runtimes (OpenMP, sanitizers), the addArchSpecificRPath() is being called. The same thing should happen when linking with the Fortran runtime, this will improve user experience massively.

…cificRPath() should be called too

When linking with other runtimes (OpenMP, sanitizers),
the addArchSpecificRPath() is being called. The same thing should
happen when linking with the Fortran runtime, this will improve
user experience massively.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' flang:driver flang Flang issues not falling into any other category labels Nov 4, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 4, 2024

@llvm/pr-subscribers-flang-driver
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Paul Osmialowski (pawosm-arm)

Changes

When linking with other runtimes (OpenMP, sanitizers), the addArchSpecificRPath() is being called. The same thing should happen when linking with the Fortran runtime, this will improve user experience massively.


Full diff: https://github.com/llvm/llvm-project/pull/114837.diff

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+1)
  • (modified) flang/test/Driver/arch-specific-libdir-rpath.f95 (+7)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 1c3c8c816594e5..ca06fb115dfa11 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1293,6 +1293,7 @@ void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args,
     }
     CmdArgs.push_back("-lFortranRuntime");
     CmdArgs.push_back("-lFortranDecimal");
+    addArchSpecificRPath(TC, Args, CmdArgs);
   }
 
   // libomp needs libatomic for atomic operations if using libgcc
diff --git a/flang/test/Driver/arch-specific-libdir-rpath.f95 b/flang/test/Driver/arch-specific-libdir-rpath.f95
index cc09938f7d1e28..23fb52abfbd574 100644
--- a/flang/test/Driver/arch-specific-libdir-rpath.f95
+++ b/flang/test/Driver/arch-specific-libdir-rpath.f95
@@ -16,6 +16,13 @@
 !
 ! Test that -rpath is added
 !
+! Add LIBPATH, RPATH
+!
+! RUN: %flang %s -### --target=x86_64-linux \
+! RUN:     -resource-dir=%S/../../../clang/test/Driver/Inputs/resource_dir_with_arch_subdir \
+! RUN:     -frtlib-add-rpath 2>&1 \
+! RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
+!
 ! Add LIBPATH, RPATH for OpenMP
 !
 ! RUN: %flang %s -### --target=x86_64-linux -fopenmp \

Copy link
Member

@Meinersbur Meinersbur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addSanitizerRuntime and addOpenMPRuntime are already calling addArchSpecificRPath. It it a problem if rpath is added multiple times?

Compiler-rt libs (libclang_rt.*.a) are added as absolute paths, shouldn't the Fortran-runtime do the same?

@pawosm-arm
Copy link
Contributor Author

addSanitizerRuntime and addOpenMPRuntime are already calling addArchSpecificRPath. It it a problem if rpath is added multiple times?

I've been testing it with and without -fopenmp and didn't observe any problem

Compiler-rt libs (libclang_rt.*.a) are added as absolute paths, shouldn't the Fortran-runtime do the same?

This makes sense for static libraries, Fortran-runtime can be built static or shared, and in case of shared lib, the rpath issue starts to kick in.

Copy link
Member

@Meinersbur Meinersbur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pawosm-arm
Copy link
Contributor Author

addSanitizerRuntime and addOpenMPRuntime are already calling addArchSpecificRPath. It it a problem if rpath is added multiple times?

For a reference, in case of Clang and C code utilizing OpenMP, -rpath is given at least twice: "/usr/bin/ld" .... "-rpath" "/opt/llvm/lib/clang/20/lib/aarch64-unknown-linux-gnu" "-rpath" "/opt/llvm/bin/../lib/aarch64-unknown-linux-gnu" "-L/opt/llvm/lib"

@pawosm-arm
Copy link
Contributor Author

pawosm-arm commented Nov 5, 2024

If there are no objections by others, I'll merge it some time tomorrow.

@pawosm-arm pawosm-arm merged commit 3c4e6c1 into llvm:main Nov 6, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category flang:driver flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants